[Part 2] SDL3: display+window: runtime fixes #3577
Conversation
ankith26
commented
Sep 1, 2025
ankith26
commented
- display index compat code
- fullscreen compat code unified
- syncwindow calls so that changes take immediate affect like sdl2
📝 WalkthroughWalkthroughAdds SDL3-focused updates: a boolean GL swap-interval wrapper, primary-display resolution in display APIs, a unified boolean fullscreen helper, and widespread SDL_SyncWindow calls after window/display state changes. Adjusts message box return checks and refactors GL attribute set/get wrappers for SDL3 semantics. SDL2 paths remain guarded and compatible. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Display as Display API
participant SDL as SDL (SDL2/SDL3)
participant Win as SDL_Window
rect rgba(200,230,255,0.3)
note over App,SDL: Set video mode / toggle fullscreen (SDL3 path)
App->>Display: pg_set_mode(...) / pg_toggle_fullscreen(...)
Display->>SDL: PG_SetWindowFullscreen(Win, fullscreen, non_desktop)
alt SDL3
SDL-->>Display: bool success/failure
Display->>SDL: SDL_SyncWindow(Win)
else SDL2
SDL-->>Display: int (0 on success)
end
Display-->>App: result
end
rect rgba(220,255,220,0.3)
note over App,SDL: VSync configuration (SDL3 path)
App->>Display: set vsync (-1/0/1)
Display->>SDL: PG_GL_SetSwapInterval(interval)
SDL-->>Display: bool success/failure
Display-->>App: result
end
rect rgba(255,240,200,0.3)
note over App,SDL: Primary display resolution (SDL3 path)
App->>Display: list_modes/mode_ok/get_video_info
Display->>SDL: SDL_GetPrimaryDisplay()
SDL-->>Display: primary_display index/handle
Display->>SDL: Query modes for primary_display
SDL-->>Display: modes / errors
Display-->>App: data or raised error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src_c/_pygame.h(2 hunks)src_c/display.c(31 hunks)src_c/window.c(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Starbuck5
PR: pygame-community/pygame-ce#3573
File: src_c/_camera.c:129-131
Timestamp: 2025-08-30T21:11:00.196Z
Learning: When doing SDL2 to SDL3 compatibility changes, Starbuck5 prefers to maintain exact existing behaviors even when other improvements could be made, focusing solely on the compatibility aspect rather than mixing in behavioral fixes.
📚 Learning: 2025-08-30T21:11:00.196Z
Learnt from: Starbuck5
PR: pygame-community/pygame-ce#3573
File: src_c/_camera.c:129-131
Timestamp: 2025-08-30T21:11:00.196Z
Learning: When doing SDL2 to SDL3 compatibility changes, Starbuck5 prefers to maintain exact existing behaviors even when other improvements could be made, focusing solely on the compatibility aspect rather than mixing in behavioral fixes.
Applied to files:
src_c/display.c
🧬 Code graph analysis (1)
src_c/display.c (2)
src_c/base.c (1)
pg_GetDefaultWindow(1916-1920)src_c/_pygame.h (1)
PG_GL_SetSwapInterval(384-388)
🔇 Additional comments (22)
src_c/window.c (4)
304-313: LGTM! Proper SDL3 boolean return handlingThe conversion to handle SDL3's boolean return value for
SDL_SetWindowFullscreenis correct, including the addition ofSDL_SyncWindowfor immediate effect.
349-349: Good addition of window sync for SDL3Adding
SDL_SyncWindowafter fullscreen setup ensures the changes take immediate effect, maintaining consistency with SDL2 behavior.
382-382: Correct type change for SDL3 compatibilityChanging
input_onlyfromSDL_booltointis appropriate since it's now used as a Python boolean viaPyArg_ParseTupleAndKeywords.
434-436: Consistent SDL_SyncWindow additionsAll the
SDL_SyncWindowcalls are properly guarded withSDL_VERSION_ATLEAST(3, 0, 0)and placed after window state changes. This ensures immediate synchronization with the windowing system in SDL3.Based on the learnings provided, this follows the preferred approach of maintaining exact existing behaviors for SDL2 to SDL3 compatibility changes.
Also applies to: 444-446, 454-456, 759-761, 814-816, 857-859, 891-893
src_c/display.c (18)
282-286: Proper null window handlingGood defensive programming - checking if the window exists before accessing its flags prevents potential crashes.
467-472: Correct primary display handling for SDL3The code properly handles SDL3's primary display API where display ID 0 has special meaning. The error checking when
SDL_GetPrimaryDisplay()returns 0 is essential.
813-821: SDL3 boolean return value handling looks correctThe GL attribute setter properly handles SDL3's boolean return convention.
833-841: GL getter boolean handling is correctThe GL attribute getter properly handles SDL3's boolean return convention.
1140-1192: Comprehensive fullscreen wrapper implementationThe
PG_SetWindowFullscreenfunction properly abstracts the differences between SDL2 and SDL3 fullscreen handling, including the boolean return values and window synchronization.
1270-1278: Primary display compatibility for SDL3Good implementation of backwards compatibility - SDL2's display index 0 is properly mapped to SDL3's primary display.
1483-1491: Display bounds API adaptationCorrectly handles the different return conventions between SDL2 and SDL3 for
SDL_GetDisplayUsableBounds.
1580-1584: Consistent use of PG_SetWindowFullscreen wrapperAll fullscreen state changes properly use the new wrapper function, maintaining consistency throughout the codebase.
1642-1661: Swap interval wrapper usageGood use of the
PG_GL_SetSwapIntervalwrapper to handle SDL3's boolean return convention for vsync control.
1899-1901: Window synchronization pointsAll SDL_SyncWindow calls are properly placed after window state changes and correctly guarded with SDL3 version checks.
Also applies to: 2000-2002, 2794-2796
2030-2038: Primary display handling in mode_okConsistent implementation of the primary display compatibility layer for SDL3.
2065-2073: Smart compatibility fallbackThe code cleverly uses the current display mode when the resolution matches, providing better SDL2 compatibility. This is a nice touch that maintains expected behavior.
2113-2121: Primary display handling in list_modesConsistent primary display compatibility implementation.
2133-2139: Proper bpp fallback using current modeGood use of current display mode for bits-per-pixel when not specified, maintaining SDL2 behavior.
2143-2157: Robust empty list handlingExcellent defensive programming - when SDL3 returns an empty display modes list (which SDL2 didn't), the code falls back to using the current mode. This maintains backwards compatibility.
3260-3527: Comprehensive fullscreen toggle implementationThe toggle_fullscreen function properly uses
PG_SetWindowFullscreenthroughout and addsSDL_SyncWindowat the end for SDL3. The implementation maintains compatibility across different rendering backends (GL, SDL_Renderer, software).
3605-3607: Display resize event synchronizationProper addition of
SDL_SyncWindowafter window size changes in the resize event handler.
3841-3844: Message box return value handlingCorrectly inverts the return check for SDL3's
SDL_ShowMessageBoxwhich returns true on success instead of 0.
